Skip to content

【Task.31】feat(sft): add pair-aware DPO training - #257

Open
A-Words wants to merge 24 commits into
redai-studio:mainfrom
A-Words:feat/task-31-dpo
Open

A-Words wants to merge 24 commits into
redai-studio:mainfrom
A-Words:feat/task-31-dpo

Conversation

@A-Words

@A-Words A-Words commented Aug 11, 2026 •

Copy link
Copy Markdown
Contributor

What

本 PR 在现有离线 SFT 链路中加入 pair-aware Direct Preference Optimization(DPO)能力。

  • 增加严格的 chosen/rejected 偏好数据 schema 校验、渲染、截断、拒绝原因统计,以及确定性的 UltraFeedback 子集生成。
  • 在 TransferQueue、SeqlenBalancedSampler、DP 分配、动态 batching 和 micro-batch packing 全链路保持 preference pair 原子性。
  • 增加标准 reference DPO、reference-free DPO loss、pair-count 归一化和 DPO 指标。
  • 增加事务化 frozen-reference 重建,并校验 canonical parameter、源文件 manifest、optimizer state 和固定 probe digest。
  • 将验证后的 reference identity 写入 checkpoint sidecar,并在 fresh/resume 训练中持续校验。
  • 增加公开的 Qwen3-0.6B/UltraFeedback recipe、中英文文档和聚焦测试。

本 PR 不增加 Reward Model scalar head。Reward Modeling 和最终的共享偏好评测器位于后续 PR #258。

Why

Relax 当前主要覆盖在线 RL 工作流。Task 31 要求在不复制训练框架的前提下,复用 SFT trainer 建立标准的离线偏好优化链路。

实现遵循 RFC #208 和Task 31 要求。

Relates to #208.

Follow-up: #258.

How

本次更新合并主线 0651812093e3cd730302709b3db35ce0bb199e4b,保留作者历史;主线集成提交为 14ccc2a。

每个 TransferQueue row 表示一个完整 preference pair,并通过 custom_meta 携带 chosen/rejected 的合并 sequence cost。固定版本的 SeqlenBalancedSampler 因而可以在不拆分 pair 的前提下形成等大小 DP 分组。动态 packer 先分配完整 pair,仅在构造模型输入时展平 chosen/rejected branch。

DPO loss 消费显式的 pair/branch identity,使用 completion-only mask,按全局 pair 数归一化,同时支持标准 reference 和 reference-free 公式。空 mask、非有限值、prompt prefix 不一致、截断后两侧相同、超大 pair,以及不一致的 global pair count 均 fail closed。

标准 DPO fresh 训练将初始 actor 快照为 frozen reference;resume 时则从固定 HF repository/revision 执行 model-only reference 重建。Canonical parameter、源文件 manifest、optimizer master/state 和固定 log-prob probe digest 可防止 reference 切换静默修改 actor 或 optimizer。验证后的 identity 会写入 checkpoint 旁的 relax_dpo_reference.json。

Testing

删除源码字符串断言(aab0c72)

  • 删除 test_preference_iterator_has_no_device_scalar_readback 及其 inspect 导入,保留 pair batching、DP 分配和分母校验的行为测试。
  • 全量 pre-commit 通过;本次仅删除测试,未在本地重跑需 Megatron/CUDA 的 GPU 测试,新提交的 CI 结果以 checks 为准。

CI 测试修正(4bc9dda)

  • 已回退 CPU CI stub 特判提交,删除 test_pinned_seqlen_sampler_consumes_pair_costs_and_keeps_equal_dp_groups 及固定源码 SHA-256 检查。
  • missing-media 测试在启动后台 prefetch 前安装 warning 捕获,消除捕获顺序竞态。
  • 使用当前 CPU CI 的 TransferQueue stub 执行 preference utils 与 streaming dataset 回归:66 passed;全量 pre-commit 通过。
  • 最新提交的 GitHub CI 结果以本 PR checks 为准。

主线集成提交 14ccc2a 的本地验证

  • pre-commit run --all-files --show-diff-on-failure:通过。
  • Python 3.12 CPU 聚焦回归:234 passed, 7 skipped。
  • 覆盖 SFT engine / producer / sharding、偏好数据截断与 split/resume、独立数值 loss、reference identity、参数校验和 Docker 固定版本 TransferQueue sampler。
  • 跳过项需要 Megatron/CUDA,包括生产后端 loss/batching 和部分 reference/训练数据路径。本次未执行完整 pytest tests/,未重跑 GPU 训练、DP=2 或 distributed checkpoint save/resume;最新 CI 以本 PR checks 为准。
python -m pytest tests/engine/sft/ tests/components/test_sft.py \
  tests/backends/megatron/test_preference_batching.py \
  tests/backends/megatron/test_dpo_loss.py \
  tests/backends/megatron/test_dpo_reference_integrity.py \
  tests/backends/megatron/test_sft_train_data_fields.py \
  tests/utils/training/test_preference_utils.py \
  tests/data/test_prepare_ultrafeedback_preferences.py \
  tests/utils/test_arguments_sft.py -q -ra --tb=short

作者提供的历史 GPU 验证

以下日志、曲线和证据包对应旧集成提交,保留用于追溯,不代表本次合并后的 GPU 复测结果。主线已更新 shuffle 行为,重新验证时应比较新版本的连续训练与恢复训练。

在最终堆叠集成 head b57a1818f2f80d28d0ad650b314eb4d1c82df3aa 的标准容器中执行:

pytest tests/engine/sft/ \
  tests/backends/megatron/test_sft_train_data_fields.py \
  tests/utils/training/test_preference_utils.py \
  tests/backends/megatron/test_preference_batching.py \
  tests/backends/megatron/test_dpo_loss.py \
  tests/backends/megatron/test_reward_model_loss.py \
  tests/backends/megatron/test_reward_model_checkpoint.py

179 passed
pre-commit run --all-files
All hooks passed

固定 TransferQueue 兼容性测试覆盖 pair-row 原子性、等大小 DP 分组、row-aligned total_lengths cost 消费,以及无丢失、无重复的完整分配。数据准备脚本的测试覆盖 manifest:4,096 个 train pair、512 个互不重叠的 eval pair、确定性选择、拒绝原因计数和派生文件 hash。

GPU 验证固定使用相同的模型/data revision 和 global batch size 32 pairs:

运行 Steps Final loss Margin Tie-aware accuracy Bootstrap lower 95%
DPO DP=1 200 0.630386 0.186692 0.654297 0.619141
DPO DP=2 200 0.628974 0.190042 0.664062 0.628906
  • DP=1 resume 从 iteration 199 恢复,完成下一 step,并保持相同的 reference identity。
  • Reference-free DPO 完成 20 steps,没有 reference log-prob 指标、reference checkpoint load 或 reference sidecar。
  • DP=2 日志明确记录 SeqlenBalancedSampler with dp_size=2,两张 A100 均有实际活动。

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • CI/CD or build changes

Screenshots / Logs

本 PR 附带 DPO DP=1/DP=2 训练曲线和脱敏后的可复现性证据包。证据包包含:

  • 200-step 训练日志、scalar CSV 和 GPU telemetry;
  • step 0/final 逐 pair 评测结果与冻结 batch plan;
  • DP=1 resume、reference-free DPO、probe contract 和 reference sidecar;
  • 固定环境、模型/data revision、镜像身份和逐文件 SHA-256 清单。

公开附件不包含 checkpoint、optimizer state、模型权重、完整数据集、凭据或机器专属标识。

测量环境使用 ghcr.io/redai-infra/relaxrl:dev-20260723-8cc1e8fd;证据采集时从 GHCR 解析的 manifest digest 为 sha256:8dc39af377a570e6cd7ec88c8b7fcd44c1eb820111e9d2069f1c7c3024b2ea23。Transformer Engine 无法导入顶层 flash_attn_interface,并明确回退到 native attention。因此 DP 对比描述的是相同 native-attention 环境,不是 FlashAttention 性能结果。

task31-pr1-dpo-evidence-public-v2.tar.gz

task31-pr1-dpo-dp1-curves task31-pr1-dpo-dp2-curves

A-Words added 12 commits August 7, 2026 20:08
# 🐛 Bug Fix

## Reject --ref-load under preference objectives

- Standard DPO snapshots the frozen reference from the initialized
  policy and rebuilds it from --hf-checkpoint on resume; --ref-load
  was silently ignored on that path while also rerouting the bridge
  mode policy-init fallback, so validate_preference_args now fails
  fast when it is set (RFC redai-studio#208: --ref-load is not the v1 DPO
  reference source)
- Drop the misleading --ref-load from the DPO recipe; behavior is
  unchanged because the bridge fallback already resolves to the same
  HF checkpoint

---

# ♻️ Refactor

## Deduplicate the preference-mode predicate

- Route data.py get_data_iterator and loss.py loss_function through
  is_preference_mode() instead of inline loss_type/sft_objective
  checks, keeping objective dispatch on a single source of truth

---

# ✅ Tests

## Cover --ref-load rejection

- Parametrize standard and reference-free DPO rejection cases in
  test_preference_runtime.py
# ♻️ Refactor

## Remove unreachable DPO loss fallback

- Drop the dead `0 * logits.sum()` branch in dpo_loss_function:
  build_preference_pair_indices already raises on an empty micro-batch

## Attach stable reason codes at preference raise sites

- Introduce _PreferenceRowError carrying an explicit reason_code from
  every normalization/split/truncation rejection; message matching in
  _classify_preference_error remains only as a fallback for errors
  raised outside the module, so reworded messages can no longer
  silently degrade rejections to "schema"

---

# ⚡ Performance

## Merge DP pair-count collectives

- Reduce _get_preference_data_iterator from three all_reduces to two
  by riding MIN/MAX on a single MAX all_reduce over [count, -count]

## Skip redundant same-tag model restores

- _switch_model now returns early when the target tag is already
  active, eliminating the duplicate full-weight CPU->GPU restore after
  the ref-forward finally block; paths that deliberately dirty weights
  already clear the tag first (covered by the injected-loader-failure
  reference integrity test)

---

# 📝 Documentation

## Note probe bitwise-determinism prerequisite

- en/zh DPO guides now state that the reference probe digest assumes
  an identical GPU/driver/image/kernel stack on resume, and that a
  probe mismatch signals environment drift rather than data corruption

## Justify the checkpoint-save barrier

- Comment that the post-sidecar barrier is functional (peers must not
  pass before rank 0 persists the identity file), and use an explicit
  gloo group for the rank-0 check per distributed code rules
# 🐛 Bug Fix

## Preserve standard DPO likelihood semantics

- Reject non-unit or non-finite rollout temperatures before actor construction
- Replace CUDA-to-Python condition checks with device-side asynchronous assertions
- Consolidate finite-value validation on the final DPO logits

## Validate preference data preparation inputs

- Validate message objects, roles, and string content on both preference branches
- Classify non-object source rows as schema rejections

---

# ✅ Tests

## Cover validation regressions

- Test finite and non-unit temperature failures
- Verify CUDA conditions avoid Python boolean conversion
- Cover malformed chosen and rejected message schemas
# 🐛 Bug Fix

## Pin standard DPO frozen reference provenance

- Resolve the reference from the declared repository and revision in the configured HF checkpoint directory
- Require Hugging Face local metadata to verify the pinned snapshot without downloading during actor startup
- Rebuild the frozen reference from that verified directory for both fresh starts and resumes
- Fail clearly when the configured checkpoint is unavailable, unverified, or resolves elsewhere

---

# 📝 Documentation

## Document pinned local model preparation

- Show the fixed-revision hf download command used by the public DPO recipe

---

# ✅ Tests

## Cover local reference resolution

- Verify repository, revision, local directory, and local-only cache resolution
- Verify missing metadata and mismatched resolved directories fail before model loading
# 🐛 Bug Fix

## Require pinned local snapshot metadata

- Require an immutable 40-character commit SHA for standard DPO references
- Verify Hugging Face local-dir tree metadata before accepting the configured checkpoint
- Reject copied or incomplete local snapshots that lack pinned provenance metadata

---

# ✅ Tests

## Cover metadata validation

- Exercise the pinned tree lookup and missing-metadata rejection paths
- Document the full commit-SHA requirement for the DPO recipe
# 🐛 Bug Fix

## Validate pinned Hugging Face local snapshots

- Parse the per-file metadata format used by pinned huggingface_hub 1.7.2
- Require every reference file to match the configured commit revision
- Verify regular Git files with Git blob SHA-1 and LFS files with SHA-256
- Stream large-file hashing to keep reference validation memory bounded

---

# ✅ Tests

## Cover reference provenance and integrity

- Exercise Git and LFS ETag validation
- Reject missing or mismatched local metadata
- Reject replaced weights even when their original mtime is restored
# 🐛 Bug Fix

## Enforce local reference completeness

- Require a supported single-file weight or standard Transformers weight index
- Validate non-empty index weight maps, safe shard paths, and format-specific suffixes
- Reject indexes that reference missing model shards

---

# ✅ Tests

## Cover missing reference weights

- Accept a complete multi-shard safetensors index
- Reject snapshots without a supported weight entry point
- Reject a deleted shard even when its local metadata is also removed
# 🐛 Bug Fix

## Move preference scheduling to the CPU control plane

- Replace CUDA count reductions and scalar readbacks with one DP-Gloo control gather
- Preserve pair-row, global-denominator, and micro-batch-count agreement across ranks
- Require Gloo process groups for preference objectives and enable them in the DPO recipe

---

# ⚡ Performance

## Remove hot-path GPU synchronization

- Eliminate all item calls and control tensors from the preference iterator
- Avoid GPU-to-CPU synchronization for train, reference, and evaluation iterators

---

# ✅ Tests

## Guard preference iterator contracts

- Cover unequal DP pair-row rejection through the Gloo control path
- Assert the iterator contains no item or all-reduce scalar readback path
# ♻️ Refactor

## Keep one reference identity path

- Remove the unused helper that reads the already-backed-up ref weights
- Preserve rebuild validation and backup ordering without duplicate digest work

---

# ✅ Tests

- Pass the full pre-commit suite
- Compile the Megatron actor module
@A-Words A-Words changed the title feat(sft): add pair-aware DPO training 【Task.31】feat(sft): add pair-aware DPO training Aug 11, 2026
@A-Words
A-Words marked this pull request as ready for review August 11, 2026 05:55
@yxyOo

yxyOo commented Aug 21, 2026

Copy link
Copy Markdown
Member

Nit: Please avoid committing data JSON files directly to this PR. Consider adding a download link instead.

@A-Words

A-Words commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the suggestion. I removed the checked-in UltraFeedback manifest from this PR, updated the DPO documentation to use the reproducibility evidence bundle, and updated the evidence bundle linked in the PR description to include the manifest.

# 🐛 Bug Fix

- Merge current upstream main while preserving the original DPO history.
- Reuse the shared SFT dataset factory and pair-aware queue payload path.
- Preserve async SFT producers, sequence classification and MTP behavior;
  reject those unsupported combinations for preference objectives.
- Add deterministic preference split/resume interfaces and propagate the
  actual global pair count to scheduler and metric normalization.

# ✅ Tests

- Focused CPU suite: 234 passed, 7 skipped (Megatron/CUDA unavailable).
- Exercise the Docker-pinned TransferQueue pair sampler without Megatron.
- pre-commit run --all-files --show-diff-on-failure passed.
- GPU training and distributed save/resume were not rerun.

# 📝 Documentation

- Document the v1 compatibility boundaries in English and Chinese.

Co-authored-by: Codex <noreply@openai.com>
@SigureMo SigureMo self-assigned this Sep 21, 2026
@rai-studio-bot

rai-studio-bot commented Sep 21, 2026 •

Copy link
Copy Markdown

Nyanpasu 审查看板

审查状态: 🚧 需要修改

审查版本: 67697fd

67697fd 复查完成:本提交落实 F4(actor.py:1811 复用 _is_standard_dpo(),成为唯一判定来源)与 F5(删除 test_preference_batching.py 中重复的 packer 纯函数测试,oversize 断言以更严格形式合并入 test_preference_utils.py),均已验证并回复原线程;F1(P1)与 F2(P2)涉及文件未变、仍待处理;CI 8 项全部通过(含 Python 3.10–3.12 与 H20 4GPU 单测)。结论:仍需修改(F1 为阻塞项)。

审查阶段进度范围与结果
常规审查 ✅ 已完成 已审查 67697fd 相对 e8d5716 的增量(3 文件、+2/-19):核对 actor.py 判定复用替换、test_preference_batching.py 两个直接导入 packer 的测试删除与导入清理、test_preference_utils.py oversize 断言收紧;确认 F1/F2 涉及的 relax/engine/sft/ 与 relax/utils/ 文件逐字节未变;CI 8 项全部通过。
深度审查 ✅ 已完成 本轮 delta 仅为落实前一轮发布的精简决定(F4/F5),无新增生产逻辑;生产/测试必要性审计沿用前一轮结论并逐项核实落地:F4(replace)已实施且 _is_standard_dpo() 为唯一判定来源,F5(remove)已实施且保留更严格断言;reference 校验链与 loss/batching 的 retain 决定不受影响。无独立设计子任务(决定记录于 review-plan-round2.json)。

审查发现

待处理
编号 严重性 问题状态规则来源
F1 High severity 跨 epoch 重复采样被误判为 pair ID 冲突 🚧 未解决 —
F2 Medium severity generation marker 模板未排除历史 assistant mask 🚧 未解决 —
已解决或已取代
编号 严重性 问题状态规则来源
F3 Medium severity 真实 sampler 校验在 CPU CI stub 上必然失败 ✅ 已解决 —
F4 Low severity 复用 _is_standard_dpo() 判定,删除内联副本
精简建议(非阻塞)
✅ 已解决 —
F5 Low severity 去重 pair packer 纯函数测试
精简建议(非阻塞)
✅ 已解决 —
提交范围 · 接收 28 · 建议移出 0 · 待确认 0

接收 28 个文件 · 建议移出 0 个文件 · 待确认 0 个文件。移出与待确认部分暂停深审,不代表审查通过。

文件结论仓库维护必要性依据替代去向或方案
relax/backends/megatron/actor.py
relax/backends/megatron/checkpoint.py
relax/backends/megatron/data.py
relax/backends/megatron/loss.py
relax/backends/megatron/model.py
relax/backends/megatron/reference_integrity.py
relax/components/actor.py
relax/components/sft.py
relax/engine/sft/bootstrap.py
relax/engine/sft/dataset/preference.py
relax/engine/sft/runtime.py
relax/utils/arguments.py
relax/utils/data/stream_dataloader.py
relax/utils/training/data_fields.py
relax/utils/training/preference_utils.py
接收 Task 31 要求在现有 SFT trainer 内实现离线 DPO 训练链路(偏好数据校验/截断、pair 原子 batching、DPO loss、frozen reference 完整性与 checkpoint sidecar)。这些文件被生产训练入口(train.py -> components/sft.py -> engine/sft -> backends/megatron)实际调用,属于必须随仓库维护的生产行为。 Task 31 要求 https://github.com/redai-infra/community/blob/main/contributor-program/2026-cohort-1/official-task.md 与 RFC #208;调用链 relax/entrypoints/train.py -> relax/components/sft.py -> relax/engine/sft/runtime.py -> relax/backends/megatron/actor.py。 不放入仓库则 DPO 目标无法通过现有 SFT trainer 训练;外部实验脚本无法满足服务契约,也不满足任务要求的可复用离线偏好优化链路。
scripts/data/prepare_ultrafeedback_preferences.py
scripts/training/dpo/run-qwen3-0.6B-ultrafeedback-1xgpu.sh
接收 公开可复现的 Qwen3-0.6B/UltraFeedback DPO recipe 是 Task 31 的交付物之一。启动脚本遵循仓库 scripts/training/<objective>/ 既有惯例(sft/genrm/diffusion 等),数据准备脚本有独立回归测试 tests/data/test_prepare_ultrafeedback_preferences.py。 PR 描述(Task 31 公开 recipe)与仓库惯例 scripts/training/genrm/run-qwen3-4B-8xgpu-genrm.sh;AGENTS.md 将 scripts/ 列为标准结构。 放到 PR 证据归档会失去可维护性与可复现性;仓库无其他数据准备脚本目录,scripts/data/ 与现有布局一致。
tests/backends/megatron/test_dpo_loss.py
tests/backends/megatron/test_dpo_reference_integrity.py
tests/backends/megatron/test_preference_batching.py
tests/backends/megatron/test_sft_train_data_fields.py
tests/data/test_prepare_ultrafeedback_preferences.py
tests/engine/sft/dataset/test_preference.py
tests/engine/sft/test_preference_runtime.py
tests/utils/training/test_preference_utils.py
接收 为新增 DPO 生产链路提供回归覆盖:loss 数值/归一化、pair batching 原子性、reference identity、数据 schema/截断/split 与恢复。均对应生产行为,不是模拟器的自测。 Task 31 验证要求与仓库 tests/ 结构惯例(tests/backends/megatron、tests/engine/sft、tests/utils)。 仅保留 GPU 端到端测试无法在 CI 上守护 CPU 可达的截断/校验路径;删除会留下真实回归风险。
docs/.vitepress/config.mts
docs/en/guide/dpo-training.md
docs/zh/guide/dpo-training.md
接收 中英双语 DPO 使用文档是任务显式交付物;结构遵循 docs/{zh,en}/guide/ 与 sft-training.md/ppo-training.md 既有模式,并注册进 VitePress sidebar。 Task 31 要求(中英文文档)与仓库双语文档惯例 docs/en/guide/sft-training.md、docs/zh/guide/。 无:文档描述的均为本 PR 合入的生产功能,外部归档无法随代码演进维护。
精简审查与验证依据
审查范围进度结论
生产代码 ✅ 已完成 前一轮审计决定的实施已核验:F4(replace,复用 _is_standard_dpo())已实施于 67697fd;reference 校验链与 loss/pair batching 的 retain 决定不受本提交影响(相关文件未变)。
测试 ✅ 已完成 前一轮审计决定的实施已核验:F5(remove,删除重复 packer 纯函数测试)已实施于 67697fd;oversize 断言以更严格形式保留在 test_preference_utils.py,其余分层测试不受影响。

生产代码的必要性与替代方案

范围必须保留的契约更简单的方案结论依据与限制
relax/backends/megatron/actor.py:1811-1813 内联 standard DPO 判定 standard DPO(非 reference-free)判定需在训练与 checkpoint 路径一致;权威实现在 actor.py:766 的 _is_standard_dpo(),调用方为 train_actor 与 save_checkpoint。 直接调用 self._is_standard_dpo(),删除内联副本。 替换 方法与内联逐字相同(is_preference_mode + sft_objective=='dpo' + 非 dpo_reference_free);已在 67697fd 实施替换(actor.py:1811 改为 self._is_standard_dpo()),全文件四处调用统一,CI 8 项全部通过。F4 线程已回复验证并标记解决。
relax/backends/megatron/reference_integrity.py 与 actor.py 的 frozen-reference 校验链(pinned 解析、事务化重建、参数/optimizer/probe digest、sidecar) Task 31 / RFC #208 要求 resume 安全的 frozen reference:resume 从固定 repository/revision 重建且不静默修改 actor/optimizer;调用方为 MegatronTrainRayActor 初始化、train_actor 与 save_checkpoint。 直接信任 load_checkpoint,省去 digest/probe/sidecar 校验(约 500 行)。 保留 该校验链是前几轮评审结论的直接产物(enforce reference integrity、pin frozen snapshot 等 5 个修复提交),非推测性兼容;删除会破坏 PR 自身交付契约(reference sidecar 持续校验)。CPU 侧 17 个完整性测试在 CI 通过。
relax/utils/training/preference_utils.py 纯函数 + relax/backends/megatron/data.py pair 展开/DP 迭代器 + loss.py dpo_loss_function pair 原子性贯穿 TransferQueue → DP 分配 → packing → loss;dpo_pair_loss 为唯一 loss 公式实现。 在 loss.py 内联 loss 公式或复制 pair 展开逻辑。 保留 已核对无重复实现:dpo_loss_function 只消费 preference_utils 的公式与索引构建;expand_preference_rollout_data 幂等(guard preference_pair_costs);跨 rank gloo 检查为分布式 fail-closed 守卫而非冗余分支。

测试的必要性与替代方案

范围必须保留的契约更简单的方案结论依据与限制
tests/backends/megatron/test_preference_batching.py::test_capacity_packer_is_deterministic_complete_and_bounded pack_preference_pair_indices 的确定性、完整覆盖与容量上界需有回归守护。 删除本副本,保留 tests/utils/training/test_preference_utils.py::test_pair_packer_is_deterministic_complete_and_capacity_safe(函数所在模块的同输入同断言测试)。 删除 两个测试导入同一纯函数、相同输入与相同断言,逐行比对确认重复;已在 67697fd 删除 test_preference_batching.py 中的两个直接导入 packer 的测试,oversize 错误信息断言(cost 11, capacity=10)合并入 test_pair_packer_reports_oversize_pair,CI(Python 3.10–3.12)全部通过。F5 线程已回复验证并标记解决。
test_dpo_loss.py 与 test_preference_utils.py 的 loss 双层测试、test_dpo_reference_integrity.py 的 17 个用例 纯公式(独立参考实现对照)与生产 loss 接线(pair identity/mask/对齐)属不同失效模式;reference 完整性各校验点各需一个失效注入。 合并为单层测试或抽稀完整性用例。 保留 两层测试断言对象不同(公式数值 vs 生产 batch 字段接线),删除任一层都会失去对应检测力;完整性用例分别覆盖 mtime 恢复、半写失败、schema 损坏等互斥场景,未见冗余断言。
Powered by Nyanpasu with glm-5.3[1m] xhigh, please check the suggestions carefully.

@rai-studio-bot rai-studio-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

需要修改:合法数据在跨 epoch 组 batch 时可能终止训练,详见行级意见;另请处理多轮 prompt 的 completion mask 和 CPU CI 的 sampler 测试问题。

已审查当前提交的数据、pair batching/loss、reference 恢复与 checkpoint 链路。本地聚焦测试共 74 passed、8 skipped;扩大 SFT 测试范围时因缺少 tensordict 无法收集,未执行本地 GPU/多节点训练。当前 GPU 检查通过,Python 3.10–3.12 检查失败,具体原因见测试行级意见。

Powered by Nyanpasu with gpt-6-astra medium, please check the suggestions carefully.

Comment on lines +536 to +540
encoded_pair_ids = [
int.from_bytes(hashlib.sha256(pair.pair_id.encode()).digest()[:8], "big") >> 1 for pair in pairs
]
if len(set(encoded_pair_ids)) != len(encoded_pair_ids):
raise ValueError("preference pair ID hash collision within batch")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 优先级:P1

请区分数据源 pair ID 和本次采样的 pair occurrence ID。现有 IndexManager.get_next_indices() 会跨 epoch 补齐 batch,因此合法数据也可能在同一个 batch 中再次采到同一行;这里却将其判为 hash collision 并终止生产。当前提交用 10 个唯一 pair、seed=42、batch size=4,即可在第 3 个 batch 复现 [pair-2, pair-3, pair-7, pair-2] 后抛错。这会使不能整除 batch size 的普通数据集在跨 epoch 时中断训练。请为每次采样分配独立的训练 pair identity(同时保留源 ID),并补充跨 epoch 的打包/训练回归;仅删除这里的检查仍会触发下游重复 chosen/rejected branch 检查。

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已在合并主线后的 e8d5716 上复核本问题:relax/engine/sft/dataset/preference.py 与 IndexManager 所在的 relax/utils/data/streaming_dataset.py 在本次主线合并中逐字节未变(合并对 PR 自有文件仅要求 actor.py 适配 device.py 重构),因此跨 epoch 重复采样触发 pack_preference_pairs_for_tq 冲突终止、以及下游 build_preference_pair_indices 重复 branch 检查的行为在新 head 上完全不变。本项仍为阻塞问题。

Comment on lines +399 to +403
chosen_tokens, chosen_mask = render_with_loss_mask(
chosen_sample,
tokenizer=self.tokenizer,
apply_chat_template_kwargs=self.apply_chat_template_kwargs,
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 优先级:P2

这里两个 render 调用需要显式只监督最终 completion。对于包含 {% generation %} 的模板,render_with_loss_mask() 默认的 assistant-mask 路径不会读取 CanonicalMessage.learn=False,而会标记所有历史 assistant 回答。共享 prompt 为 user → assistant → user 的合法 pair 因此在 _split_branch() 抛出 completion mask must be one contiguous suffix;使用本文件测试中的 tokenizer 即可复现。请在 chosen/rejected 渲染时排除历史回答(例如适用时传 last_turn_only=True),并测试带历史 assistant 的共同前缀。

Comment thread tests/utils/training/test_preference_utils.py Outdated
# ✅ Tests

- Mark the CPU CI TransferQueue import stub explicitly and skip only the
  real-sampler integration check when that stub is present. Keep the pinned
  source digest and pair-assignment assertions for actual installations.
- Install the missing-media warning capture before shuffle starts prefetch
  workers, avoiding a race in the logging assertion.
- Reproduce the exact CI stub hash failure before the fix.
- Validate 67 tests against real TransferQueue and 66 passed / 1 expected
  skip against the workflow-generated CPU stub.
- pre-commit run --all-files --show-diff-on-failure passes.

Co-authored-by: Codex <noreply@openai.com>
SigureMo and others added 2 commits September 21, 2026 16:30
# 🔩 Chore

- Revert 21d2c22 as requested before removing the pinned sampler test.
- Restore the original CI stub and test files.
- pre-commit run --all-files --show-diff-on-failure passes.

Co-authored-by: Codex <noreply@openai.com>
# ✅ Tests

- Delete the fixed-source-hash TransferQueue sampler test as requested.
- Remove its now-unused imports without adding a CI-specific skip path.
- Fix the independent prefetch logging race by installing the capture hook
  before starting the worker thread.
- Focused CPU regression: 66 passed with the CI TransferQueue stub.
- pre-commit run --all-files --show-diff-on-failure passes.

Co-authored-by: Codex <noreply@openai.com>
Comment thread tests/backends/megatron/test_preference_batching.py Outdated
# ✅ Tests

- Remove the source-string test that forbids item and all_reduce calls.
- Remove the unused inspect import.
- Keep the existing pair batching and denominator behavior tests.
- All pre-commit hooks pass.

Co-authored-by: Codex <noreply@openai.com>
# 🐛 Bug Fix

- Merge main at 204ef28 without rewriting the DPO author history.
- Keep the main warning-capture comment and the shared prefetch ordering.
- Preserve the main device, GDN context-parallel, and int32 token updates.

# ✅ Tests

- Full pre-commit checks pass with Python 3.12.
- Focused CPU regressions: 250 passed, 9 skipped using the CPU CI TransferQueue stub.
- Megatron/CUDA-specific checks skip because this host lacks patched Megatron and CUDA.

Co-authored-by: Codex <noreply@openai.com>

@rai-studio-bot rai-studio-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

仍需修改:F1(P1)在新 head 上复核仍成立

已在合并主线后的 e8d5716 上完成本轮复查:本次主线合并对 PR 自有文件仅涉及 relax/backends/megatron/actor.py(为主线 device.py 重构所做的 device_utils → device_module 机械适配,已核对正确),偏好链路 key 流(data.py → model.py → loss.py)完整,preference.py 与 IndexManager 未变,CI 8 项全部通过。既有 F1(跨 epoch 重复采样误判为 pair ID 冲突,P1)与 F2(多轮 prompt 的历史 assistant completion mask,P2)代码未变、仍待处理;F3 已解决。本轮另有两处非阻塞精简建议,见行级意见。

审查看板:#257 (comment)

Powered by Nyanpasu with glm-5.3[1m] xhigh, please check the suggestions carefully.

Comment thread relax/backends/megatron/actor.py Outdated
Comment thread tests/backends/megatron/test_preference_batching.py Outdated
# ♻️ Refactor

- Reuse _is_standard_dpo for the existing reference-forward predicate.

# ✅ Tests

- Keep pure packer coverage in the preference utility tests.
- Remove the two duplicate packer cases from the Megatron batching tests.
- Preserve the oversize error capacity assertion in the retained utility test.
- Focused CPU regression: 22 passed, 6 skipped for missing Megatron/CUDA.
- Full pre-commit checks pass.

Co-authored-by: Codex <noreply@openai.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants